Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Enable building on FreeBSD #125

Closed
wants to merge 1 commit into from
Closed

Conversation

vrachnis
Copy link

Requirements for Contributing a Bug Fix

Identify the Bug

Similar to #87, building is broken in FreeBSD for 2 reasons:

  1. The OS is not recognized by the build system.
  2. hunspell won't build cleanly under LLVM/Clang.

Description of the Change

I would really like to make this change support OpenBSD as #87 reports, but given that I don't have a VM available at the moment, I limited my changes to FreeBSD (that I can test).

This PR includes 2 changes:

  1. Make freebsd a recognizable platform alongside mac, linux and win. The logic is identical to Linux, since nothing special is required.
  2. Partially backport hunspell/hunspell@be3b8d9#diff-0dd29bfd51e6c6fdadebc9b96923c5c6 and hunspell/hunspell@dd3a71c#diff-0dd29bfd51e6c6fdadebc9b96923c5c6. Hunspell have already fixed the build issue upstream, but the version included with spellchecker is not up to date. Instead of pulling in all the changes from upstream, include the must-haves that unbreak the build.

Alternate Designs

Ideally, we could update hunspell to the latest version. However that might be a big change, given that the last time that the vendor directory was updated was 6 years ago.

With this in mind, I tried to pick the minimum amount of changes that will unbreak the build and minimize the risk of breakage.

Possible Drawbacks

I am not aware of any drawbacks. Given that this is just following the upstream commits, this shouldn't have any negative impact.

Verification Process

Verified that the build works, and that the unit tests still pass.

$ yarn test
yarn run v1.16.0
$ jasmine-focused --captureExceptions --coffee spec/
..........................................................................................................................................

Finished in 13.619 seconds
138 tests, 184 assertions, 0 failures, 0 skipped


Done in 14.37s.

Release Notes

  • node-spellchecker can now be built under FreeBSD

@vrachnis vrachnis marked this pull request as ready for review August 21, 2019 20:57
@rsese
Copy link

rsese commented Sep 17, 2019

Thanks for the contribution! We took a look at this today and it seems like this change isn't necessary for Atom specifically in that it's not fixing a problem/bug with Atom's use of this module for spellchecking (please let me know if I'm misunderstanding however).

With our current resources and priorities, as much as possible we're unfortunately not taking on changes that are for use cases outside of Atom and don't directly address some issue or bug in Atom itself. Since this isn't something the team will take on I'm going to close this out but thank you again for your contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants